Skip to content

fix(tui): 修复 /help 单页可选执行 + UTF-8 + 覆盖率修复#208

Merged
minorcell merged 31 commits into
1024XEngineer:mainfrom
creatang:codex/tui-split-04-core-app-bootstrap-cleanup
Apr 10, 2026
Merged

fix(tui): 修复 /help 单页可选执行 + UTF-8 + 覆盖率修复#208
minorcell merged 31 commits into
1024XEngineer:mainfrom
creatang:codex/tui-split-04-core-app-bootstrap-cleanup

Conversation

@creatang

@creatang creatang commented Apr 9, 2026

Copy link
Copy Markdown
Collaborator

变更说明

  • /help 改为与 /model 一致的可选择弹层展示
  • 在当前界面直接展示全部 slash 命令,不切换页面
  • 支持在 help 列表中直接选择并执行 slash 命令
  • /help 列表改为单页展示(关闭分页并自适应高度)

UTF-8 与覆盖率修复

  • 按 UTF-8 更新 PR 标题与描述,修复乱码
  • 补充 update.goview.gocopy_code.goapp.go 相关测试路径
  • 移除 refreshHelpPicker 无效错误分支,降低不可达覆盖噪音

验证

  • go test ./internal/tui/...

@codecov

codecov Bot commented Apr 9, 2026

Copy link
Copy Markdown

@creatang creatang changed the title refactor: ?? /help ??????????? fix(tui): /help ?????? + UTF-8 + ????? Apr 9, 2026
@creatang creatang changed the title fix(tui): /help ?????? + UTF-8 + ????? fix(tui): 修复 /help 单页可选执行 + UTF-8 + 覆盖率修复 Apr 9, 2026
@minorcell

Copy link
Copy Markdown
Member

/review

fennoai[bot]

This comment was marked as outdated.

fennoai[bot]

This comment was marked as outdated.

fennoai[bot]

This comment was marked as outdated.

fennoai[bot]

This comment was marked as outdated.

fennoai[bot]

This comment was marked as outdated.

xgopilot and others added 10 commits April 10, 2026 03:17
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
…t, rate-limit misclassification

- runtime.go: decrement attempt before reactive-compact continue so the
  retry does not consume a MaxLoops slot (fixes max_loops=1 case)
- compact.go: reset TokenInputTotal/TokenOutputTotal on session before
  saving after compact, preventing stale high totals from being restored
  on the next run
- errors.go: guard text-based context_too_long override to only apply for
  generic client errors (ErrorCodeClient); 429 rate-limit errors whose
  message contains token-count fragments are no longer mis-routed into
  the reactive-compact path; same guard added in IsContextTooLong
- errors_test.go: add regression cases for the 429-with-token-count-message
  scenarios in NewProviderErrorFromStatus and IsContextTooLong

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
…e-146-1775790664

feat(runtime): recover with reactive compact on context-too-long
…-ask-select-closure

feat(tui/runtime): 补齐权限 ask 的选择式闭环(上下选择 + Enter确认)
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: minorcell <120795714+minorcell@users.noreply.github.com>
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
…e-213-1775793526

fix(runtime/tools): 收敛 EmitChunk 遗留 review 问题
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: minorcell <120795714+minorcell@users.noreply.github.com>
@1024XEngineer 1024XEngineer deleted a comment from fennoai Bot Apr 10, 2026
@1024XEngineer 1024XEngineer deleted a comment from fennoai Bot Apr 10, 2026
@1024XEngineer 1024XEngineer deleted a comment from fennoai Bot Apr 10, 2026
@creatang

Copy link
Copy Markdown
Collaborator Author

/review -codex

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 2 regressions in the current diff: one in OpenAI stream completion/cancellation handling, and one in TUI layout sizing vs. hit-testing.

Comment thread internal/provider/openai/response.go Outdated
line, err := reader.ReadLine()
// 每次读取前优先响应上下文取消,避免取消请求被误判为流中断。
select {
case <-ctx.Done():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once done has been set after a [DONE] frame, this unconditional ctx.Done() check can still return context.Canceled before the trailing blank line/EOF is consumed. In that case a successfully completed stream is reported as canceled and message_done is never emitted. A regression test that cancels after [DONE] but before EOF would reproduce this.

Comment thread internal/tui/core/app/update.go Outdated
a.transcript.Height = max(6, lay.rightHeight-activityHeight-menuHeight-promptHeight)
availableHeight := lay.rightHeight - activityHeight - menuHeight - promptHeight
minTranscriptHeight := max(6, lay.rightHeight/2)
a.transcript.Height = max(minTranscriptHeight, availableHeight)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a.transcript.Height now uses max(lay.rightHeight/2, availableHeight), but renderWaterfall() still renders the transcript with max(6, height-activity-menu-prompt). When those diverge, the visual transcript/prompt positions no longer match the coordinates used by inputBounds() / activityBounds(), so mouse hit-testing and scrolling become offset on layouts where the activity pane or command menu is visible.

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two issues worth addressing before merge.

Comment thread internal/tui/core/app/update.go Outdated
a.transcript.Height = max(6, lay.rightHeight-activityHeight-menuHeight-promptHeight)
availableHeight := lay.rightHeight - activityHeight - menuHeight - promptHeight
minTranscriptHeight := max(6, lay.rightHeight/2)
a.transcript.Height = max(minTranscriptHeight, availableHeight)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a.transcript.Height is now forced to at least half of the right pane, even when the activity panel, command menu, and prompt leave less space than that. The new renderWaterfall() path recomputes a smaller visible transcript height, so the viewport's internal height diverges from what is actually rendered. On shorter terminals this throws off AtBottom/scroll calculations and can clip or jump the transcript. The viewport height should stay in sync with the rendered container instead of enforcing a larger minimum here.

Comment thread internal/runtime/runtime.go Outdated
autoCompacted = true
var compactResult contextcompact.Result
session, compactResult, _ = s.runCompactForSession(ctx, input.RunID, session, cfg, false)
session, compactResult, _ = s.runCompactForSession(ctx, input.RunID, session, cfg, contextcompact.ModeManual, false)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the new token-threshold auto-compact branch, but it still calls runCompactForSession(..., contextcompact.ModeManual, ...). As a result every compact_* event from an automatic compact is reported as manual, so the UI/docs added in this PR cannot distinguish an auto-compact from an explicit /compact. If auto-compaction is now a first-class path, it needs its own trigger mode instead of reusing manual.

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two non-blocking issues remain in the changed code paths after verification.

"maximum context length",
"maximum prompt length",
"prompt is too long",
"requested too many tokens",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"requested too many tokens" / "too many tokens" are too broad for context_too_long detection. Providers also use those phrases for bad-request cases like oversized max_tokens / requested output, which compaction cannot fix. In those cases this PR will route the error into the new reactive-compact retry path instead of surfacing the real configuration problem.

Comment thread internal/tui/core/app/view.go Outdated
activityHeight := a.activityPreviewHeight()
menuHeight := a.commandMenuHeight(width)
promptHeight := lipgloss.Height(a.renderPrompt(width))
transcriptHeight := max(6, height-activityHeight-menuHeight-promptHeight)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renderWaterfall now recomputes its own transcript height instead of using a.transcript.Height, but applyComponentLayout, inputBounds, and transcriptBounds still use a.transcript.Height. On short panes those values diverge (applyComponentLayout enforces a half-pane minimum, this code does not), which will misplace the rendered prompt/activity area relative to the viewport and break mouse hit-testing/scroll positioning.

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 2 regressions worth fixing before merge.

Comment thread internal/provider/openai/response.go Outdated
line, err := reader.ReadLine()
// 每次读取前优先响应上下文取消,避免取消请求被误判为流中断。
select {
case <-ctx.Done():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once [DONE] has been seen, this pre-read ctx.Done() check can still turn a successfully completed stream into context.Canceled if the request is canceled before the trailing blank line/EOF is consumed. In that case message_done never fires even though the model already finished. The cancel fast-path should be skipped after done == true, or handled after the pending completion is flushed.

Comment thread internal/runtime/runtime.go Outdated
@@ -251,12 +252,12 @@ func (s *Service) Run(ctx context.Context, input UserInput) error {
if builtContext.ShouldAutoCompact && !autoCompacted {
autoCompacted = true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

autoCompacted is flipped before we know whether compacting actually succeeded. Because runCompactForSession(..., failOnError=false) swallows runner/save errors and can also return Applied=false, a failed or no-op auto-compact permanently disables further threshold-triggered compaction for the rest of the run. That leaves later oversized turns with no second chance to compact.

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few regressions still need attention before this lands. The main issues are stream completion being converted into cancellation, overly broad context-too-long detection, and the new transcript sizing getting out of sync with the viewport model.

Comment thread internal/provider/openai/response.go Outdated
line, err := reader.ReadLine()
// 每次读取前优先响应上下文取消,避免取消请求被误判为流中断。
select {
case <-ctx.Done():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the provider has already sent [DONE], canceling the request before the trailing blank line / EOF now returns context.Canceled here instead of finishing the stream. That skips message_done, so the last assistant turn and token usage can be dropped during normal shutdown races. Once done is true, this pre-read cancel check should stop treating the stream as incomplete.

"maximum context length",
"maximum prompt length",
"prompt is too long",
"requested too many tokens",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new "requested too many tokens" / "too many tokens" fragments are broad enough to catch ordinary 400 validation errors that are not caused by prompt/context size (for example, asking for too many completion tokens). Those requests will now be upgraded to context_too_long and sent through the reactive-compact retry path even though compact cannot fix them.

Comment thread internal/tui/core/app/update.go Outdated
a.transcript.Height = max(6, lay.rightHeight-activityHeight-menuHeight-promptHeight)
availableHeight := lay.rightHeight - activityHeight - menuHeight - promptHeight
minTranscriptHeight := max(6, lay.rightHeight/2)
a.transcript.Height = max(minTranscriptHeight, availableHeight)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a.transcript.Height now uses max(minTranscriptHeight, availableHeight), but renderWaterfall still renders the transcript with just availableHeight. On short panes the viewport model thinks it is taller than the box actually drawn on screen, which throws off bottom detection/scroll math and causes clipping or jumpy scrolling.

@creatang

Copy link
Copy Markdown
Collaborator Author

@xgopilot 解决review提到的问题

@fennoai

fennoai Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

已完成

已修复 review 提到的几处回归,并已推送到当前 PR 分支,提交为 3fe66f0

本次处理

  • 修复 OpenAI SSE 在收到 [DONE] 后遇到取消竞争时误返回 context canceled,保证 message_done 仍能收尾发出。
  • 为阈值触发的 compact 增加独立 auto 模式,并修正只有在 auto compact 真正生效后才标记已触发,避免 no-op/失败后屏蔽后续 reactive compact。
  • 收紧 context_too_long 文本匹配,避免把 max_tokens 之类的普通 token 校验错误误归类到 compact 重试路径。
  • 统一 TUI waterfall/transcript 高度计算,消除渲染高度与 viewport / 鼠标命中区域不一致的问题。
  • 补充对应回归测试,覆盖 DONE 后取消、auto compact 模式与 no-op 退化路径、provider 错误分类、TUI 布局同步。

验证

  • go test ./internal/context/compact ./internal/provider ./internal/provider/openai ./internal/runtime ./internal/tui/core/app
  • go test ./...

View job run

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: creatang <165447160+creatang@users.noreply.github.com>
@minorcell minorcell merged commit 0e60d38 into 1024XEngineer:main Apr 10, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants